Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

front: implement macro editor OP import #7940

Merged
merged 9 commits into from
Aug 6, 2024
Merged

front: implement macro editor OP import #7940

merged 9 commits into from
Aug 6, 2024

Conversation

emersion
Copy link
Member

@emersion emersion commented Jul 4, 2024 •
edited
Loading

This PR is best reviewed commit-by-commit.

What's in the box?

  • Import all waypoints of all trains schedules as NGE nodes (whether we can find corresponding OPs for these in the infra or not, e.g. track offset or trigram which doesn't exist)
  • Import all train schedules paths
  • Import stop times specified by the user (anything left unspecified is imported as zero for now)

Closes #7737
Closes #7739
References #7741
Depends on #8006 (in other words, please ignore the first few commits, they belong to that other PR)

TODO:

  • Handle non-UIC path items
  • Prettify node coords
  • Pagination
  • The import process should only kick in when the macro tab is selected
  • Passthrough UICs/trigrams that we don't find in infra
  • Add secondary_code suffix to OPs
  • Set times based on train schedule

Follow-up work:

  • Check if rtk-query requests need to be unsubscribed
  • Adjust coords for large number of nodes (try with France infra test cases, e.g. Paris-Lyon-Grenoble)

@codecov-commenter
Copy link

codecov-commenter commented Jul 4, 2024 •
edited
Loading

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 0% with 660 lines in your changes missing coverage. Please review.

Project coverage is 36.51%. Comparing base (9c54287) to head (14182e5).

Files Patch % Lines
...perationalStudies/components/MacroEditor/import.ts 0.00% 445 Missing and 1 partial ⚠️
...operationalStudies/components/MacroEditor/types.ts 0.00% 154 Missing and 1 partial ⚠️
...operationalStudies/components/MacroEditor/utils.ts 0.00% 26 Missing and 1 partial ⚠️
.../operationalStudies/components/MacroEditor/NGE.tsx 0.00% 16 Missing and 1 partial ⚠️
...cations/operationalStudies/views/v2/ScenarioV2.tsx 0.00% 15 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files 
@@             Coverage Diff              @@
##                dev    #7940      +/-   ##
============================================
- Coverage     36.72%   36.51%   -0.21%     
  Complexity     2163     2163              
============================================
  Files          1282     1285       +3     
  Lines        119666   120313     +647     
  Branches       3218     3221       +3     
============================================
- Hits          43944    43932      -12     
- Misses        73791    74447     +656     
- Partials       1931     1934       +3
Flag Coverage Δ
core 75.59% <ø> (ø)
editoast 66.85% <ø> (-0.05%) ⬇️
front 15.53% <0.00%> (-0.15%) ⬇️
gateway 2.03% <ø> (ø)
railjson_generator 87.49% <ø> (ø)
tests 72.98% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@emersion emersion force-pushed the emersion/nge-import branch 6 times, most recently from c87330e to e033faa Compare July 4, 2024 11:53
@emersion emersion force-pushed the emersion/nge-import branch 7 times, most recently from 857bbb8 to 08d0301 Compare July 16, 2024 12:32
@emersion emersion force-pushed the emersion/nge-import branch 14 times, most recently from 0249cbd to b1c749c Compare August 1, 2024 13:48
@emersion emersion marked this pull request as ready for review August 1, 2024 13:49
@emersion emersion requested a review from a team as a code owner August 1, 2024 13:49
@emersion emersion requested review from Uriel-Sautron, SharglutDev and kmer2016 and removed request for SharglutDev August 1, 2024 13:49
kmer2016
kmer2016 reviewed Aug 2, 2024
View reviewed changes
Copy link
Contributor

@kmer2016 kmer2016 left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for breaking down your commits in this way; it makes the review process much easier.

While the content looks good, I have a few suggestions to enhance readability.

front/src/applications/operationalStudies/components/MacroEditor/import.ts Show resolved Hide resolved
front/src/applications/operationalStudies/components/MacroEditor/import.ts Show resolved Hide resolved
front/src/applications/operationalStudies/components/MacroEditor/import.ts Outdated Show resolved Hide resolved
front/src/applications/operationalStudies/components/MacroEditor/import.ts Outdated Show resolved Hide resolved
front/src/applications/operationalStudies/components/MacroEditor/import.ts Outdated Show resolved Hide resolved
front/src/applications/operationalStudies/components/MacroEditor/import.ts Show resolved Hide resolved
front/src/applications/operationalStudies/components/MacroEditor/import.ts Show resolved Hide resolved
front/src/applications/operationalStudies/components/MacroEditor/import.ts Outdated Show resolved Hide resolved
@emersion
front: add NGE DTO types
a053714
@emersion
front: add dto property to NGE component
77ba3a4
@emersion
front: listen to NGE reloads
8ce66a8 
When the NGE language is switched, the <iframe> is reloaded. Keep
listening to window load events to restore the DTO properly.
@emersion emersion force-pushed the emersion/nge-import branch 2 times, most recently from af2d61f to 5f02bf8 Compare August 2, 2024 14:04
@emersion
Copy link
Member Author

emersion commented Aug 2, 2024

Changes:

  • Fix inverted North/South in geographic coord conversion.
  • Mark non-stop transitions as such when importing stop times (isNonStopTransit).
  • Extract functions buildOpQuery and executeSearch.
  • Move findOpFromPathItem and addDurationToDate to utils.ts.

@emersion emersion requested a review from kmer2016 August 2, 2024 14:43
kmer2016
kmer2016 approved these changes Aug 5, 2024
View reviewed changes
Copy link
Contributor

@kmer2016 kmer2016 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM and tested !

Uriel-Sautron
Uriel-Sautron approved these changes Aug 5, 2024
View reviewed changes
Copy link
Contributor

@Uriel-Sautron Uriel-Sautron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good work, tested.

@emersion emersion added this pull request to the merge queue Aug 6, 2024
Merged via the queue into dev with commit 251e073 Aug 6, 2024
20 checks passed
@emersion emersion deleted the emersion/nge-import branch August 6, 2024 07:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kmer2016 kmer2016 kmer2016 approved these changes

@Uriel-Sautron Uriel-Sautron Uriel-Sautron approved these changes

@louisgreiner louisgreiner Awaiting requested review from louisgreiner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Convert train schedules/runs from OSRD to NGE, basic version Convert infrastructure from OSRD to NGE
4 participants
@emersion @codecov-commenter @kmer2016 @Uriel-Sautron